execution,vm: implement EIP-7708 (ETH transfers emit a log), active on Amsterdam - #2469
execution,vm: implement EIP-7708 (ETH transfers emit a log), active on Amsterdam#2469brett-monad wants to merge 1 commit into
Conversation
6ae040b to
d1dce79
Compare
5b7256d to
db72acf
Compare
c216e0b to
e7dee56
Compare
db72acf to
ac3bce4
Compare
e7dee56 to
e7df9b6
Compare
ac3bce4 to
23cef36
Compare
e7df9b6 to
7b8451a
Compare
23cef36 to
7a9f21c
Compare
why is there a Monad-specific extension needed? |
7b8451a to
d71adec
Compare
7a9f21c to
3ae693f
Compare
d71adec to
08a7971
Compare
3ae693f to
0d6fe6c
Compare
|
@claude review this PR |
There was a problem hiding this comment.
Pull request overview
Implements EIP-7708-style observability for native ETH movement by emitting synthetic ERC-20-like Transfer logs (and a Monad-specific Burn log for same-tx created+destroyed accounts) from SYSTEM_ADDRESS, wiring emission through the consensus execution path and updating tests/fixtures accordingly.
Changes:
- Add EIP-7708 feature-gate (
eip_7708_active) to the traits interface and trait implementations. - Emit consensus
Transfer(from,to,value)logs for non-zero native value movement (plus optional eth_simulate synthetic logs), and emitBurn(address,uint256)logs for destroyed residual balances (inline + finalization). - Change
State::destruct_suicidesto return sorted(address, balance)residuals for deterministic burn-log emission ordering; expand/adjust tests and Amsterdam fixture allowlist.
Verdict: NEEDS CHANGES
🤖 Generated with Claude Code
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/ethereum_test/exclude/MONAD_NEXT_amsterdam.cmake | Opens MONAD_NEXT+Amsterdam fixture allowlist to the full suite with updated rationale. |
| category/vm/evm/traits.hpp | Adds eip_7708_active() to the Traits contract and implementations. |
| category/rpc/monad_executor_test.cpp | Updates eth_simulate expectations to include consensus Transfer logs when 7708 is active. |
| category/execution/ethereum/test/test_call_trace.cpp | Updates call-trace tests and adds pinned Amsterdam fixtures for 7708 Transfer/Burn behavior. |
| category/execution/ethereum/state3/state.hpp | Updates destruct_suicides API to return burned residual balances for 7708. |
| category/execution/ethereum/state3/state.cpp | Captures and sorts burned residual balances during suicide destruction for deterministic Burn emission. |
| category/execution/ethereum/state2/test/test_state.cpp | Adds pinned Amsterdam test asserting burned residuals are returned sorted by address. |
| category/execution/ethereum/process_requests.cpp | Removes local SYSTEM_ADDRESS spelling in favor of shared constant. |
| category/execution/ethereum/execute_transaction.cpp | Emits finalization Burn logs based on destruct_suicides return value when 7708 is active. |
| category/execution/ethereum/execute_transaction_test.cpp | Adds pinned Amsterdam test validating finalization-site Burn log behavior. |
| category/execution/ethereum/evmc_host.hpp | Promotes SYSTEM_ADDRESS and SIMULATE_NATIVE_TOKEN_LOG_ADDRESS, adds Burn-log builder, and emits 7708 Transfer/Burn logs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
This PR adds EIP-7708 Transfer log emission from SYSTEM_ADDRESS on every value-carrying transfer (top-level tx, value-carrying CALL, SELFDESTRUCT, CREATE/CREATE2), plus a monad-specific Burn log at the two sites where an EIP-6780 same-tx-created-and-destroyed account loses ETH: inline at selfdestruct-to-self, and at finalization for residual balance. The value > 0 && from != to predicate is shared with the existing eth_simulate traceTransfers machinery; both consensus and ERC-7528 synthetic emissions coexist rather than one replacing the other, matching geth. SYSTEM_ADDRESS is promoted from a function-local constant in process_requests.cpp to a shared inline constexpr in evmc_host.hpp. Active on MonadTraits<MONAD_NEXT> (which maps to MONAD_ETH_AMSTERDAM) and on EvmTraits<MONAD_ETH_AMSTERDAM> and later.
I traced the following and found nothing wrong:
- Refactored
emit_native_transfer_event: for every combination ofeip_7708_activeandlog_native_transfers_the pre-activation truth table matches the old single-branch check; post-activation the consensus log is emitted unconditionally on the shared predicate and the synthetic still fires only under the flag. EvmcHost::selfdestructinline burn:is_current_incarnation(address)is captured beforestate_.selfdestruct(correct — the selfdestruct doesn't touch incarnation, but this reads consistently even if it did). The burn is gated oncreated_this_tx && beneficiary == address && transferred_balance > 0, and the pairedemit_native_transfer_eventself-skips onfrom == to, so no double emission. The comment "store_log without a matching on_log" documents the intentional trace/receipt asymmetry.- Finalization path:
destruct_suicidesruns after the beneficiary award, captures(address, balance)inside the CANCUN+ branch (guarded bystatic_assert(!eip_7708_active() || evm_rev() >= CANCUN)— nice), and sorts by address.execute_finalthen emits Burn logs from the returned pairs. Ordering matters and is called out in the comment. - Revert safety:
state_.store_logis versioned, so the inline burn rolls back withlogs_.pop_rejectif the emitting frame reverts. Finalization burns are at version 0 and don't need revert semantics. [[maybe_unused]] auto const burned = state.destruct_suicides<traits>();—burnedis only touched insideif constexpr (eip_7708_active()), so the attribute is necessary to silence the unused-variable warning on the discarded-branch path; NRVO keeps the returned empty vector free of overhead pre-activation.- Trait wiring: the
eip_7708_active()requirement is added to theTraitsconcept, and bothEvmTraitsandMonadTraitsadd matchingconstevalpredicates keyed onMONAD_ETH_AMSTERDAM.
Test coverage is thorough — top-level Transfer log, consensus + synthetic pairing, inline burn, finalization burn (via beneficiary award landing on a selfdestructed account), address ordering of the burned set, and the burn-out-of-call-trace invariant. All are pinned to EvmTraits<MONAD_ETH_AMSTERDAM> since no monad revision reaches AMSTERDAM in the shared matrix yet, with the !eip_7708_active() static_asserts on the two touched pre-activation tests making the intent explicit.
One meta note that doesn't block: the PR title still says "held inert on MONAD_NEXT", but the code and the commit message activate on MONAD_NEXT (via MonadTraits<MONAD_NEXT>::evm_rev() == MONAD_ETH_AMSTERDAM), and the MONAD_NEXT_amsterdam.cmake allowlist opens to "*" — consistent with activation, not inertness. Worth updating the title before merge.
All CI green (clang-format, clang-tidy, trait instantiation, Debug+ASAN, RelWithDebInfo, RISC-V, VM tests, CodeQL).
Verdict: CORRECT
🤖 Generated with Claude Code
f987246 to
5493881
Compare
a0443e9 to
0206332
Compare
5493881 to
ea14054
Compare
0206332 to
704ca18
Compare
|
Note for the rebase onto the EIP-8246 branch, which sits below this one: the Burn log becomes unreachable the moment 8246 lands.
Since 8246 lands below rather than above, the clean outcome is that the rebased version of this PR never introduces the Burn log at all, rather than adding it and deleting it a commit later. That also means the Two things to fix while rebasing: Consistent with geth, whose Amsterdam path emits only the transfer log, and only when the beneficiary differs from the destructing account. |
704ca18 to
34de85c
Compare
ea14054 to
c357443
Compare
|
Done — recording it here so nobody actions the comment above twice. EIP-8246 now sits directly beneath this PR as #2512, and this PR no longer introduces the The stale comment claiming monad had declined 8246 went with it, as did the PR title and description, which said the same thing. On the fixtures: the exclusions landed here rather than in #2512, for the reason given above — this is the PR that removes the allowlist, so this is where they become live. Measured rather than predicted: 601 of 617 pass, and the 15 that fail are exactly the set carrying the pre-8246 SELFDESTRUCT expectations. Both |
34de85c to
f8fbb2d
Compare
f927f4e to
b89479a
Compare
8f8fdc6 to
c708971
Compare
b89479a to
ff1dfbe
Compare
699bca9 to
e8a609b
Compare
ff1dfbe to
e1b9d1a
Compare
e8a609b to
34bafb7
Compare
24b9149 to
aceec06
Compare
…n Amsterdam Emit a standardised log for every native ETH movement, so indexers can track native value the way they already track ERC-20 transfers. `Transfer(address,address,uint256)` on ordinary value transfers, at the four sites Monad already wired: top-level tx transfer, value-carrying CALL, SELFDESTRUCT, and CREATE/CREATE2. That is the only log the EIP defines. There is deliberately no Burn log. An earlier draft of 7708 also specified `Burn(address,uint256)` for ETH destroyed by SELFDESTRUCT, and this commit used to implement it; the draft then took EIP-8246 as a prerequisite and dropped the log, because after 8246 there is no burn left to report. EIP-8246 is a separate branch based on main rather than a commit beneath this one, but it merges first, so by the time 7708 is live the burn never happens and the log would have no subject. geth and erigon are in the same configuration. Consensus logs use SYSTEM_ADDRESS. eth_simulate's traceTransfers keeps the ERC-7528 native-token address and is emitted alongside the consensus log rather than replacing it, second, so discarding it leaves the sequence a real block produces. Also in this commit: - Leave the spec-test configuration alone. This commit is based on the SLOTNUM branch, whose exclusion list already covers */eip7708_eth_transfer_logs/*, so 7708's own fixtures do not run yet and the Amsterdam job passes unchanged. Enabling them needs a bundle regenerated with 7708 active and is deferred to a follow-up once the whole set has landed. The base is a fixture constraint rather than a code dependency: nothing here needs anything from SLOTNUM, and this commit builds and unit-tests cleanly on main. But main pins the Amsterdam bundle at v0.2.0, which predates 7708, so every fixture that moves ETH expects no Transfer log and 110 of them break. The SLOTNUM branch carries the bump to v0.5.0, whose fixtures were generated with 7708 active. - Make the simulate_v1 trace expectations revision-aware. Four tests asserted log counts and contents that predate 7708; two stopped compiling once the rule was active and two failed at runtime. They had gone unnoticed because MONAD_NEXT only rejoined the typed-test matrices with the fork bump in the SLOTNUM commit beneath this one, so they had never run at a revision where 7708 is active. The doubling is now expressed by helpers derived from the emitter rather than from observed output. - Run the new tests over an explicit two-element type list naming both revisions where 7708 is active: MonadRevisionConstant<MONAD_NEXT> and EvmRevisionConstant<MONAD_ETH_AMSTERDAM>. That covers the configuration that ships as well as the plain-EVM one, and names the revisions rather than deriving them from LATEST_SUPPORTED_EVM_FORK, so the suite does not depend on that constant having been advanced to Amsterdam. A TypesSince form would be more durable once it has been, and is worth revisiting then. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
34bafb7 to
68a419c
Compare
Emit a standardised log for every native ETH movement, so indexers can track native value the way they already track ERC-20 transfers.
Transfer(address,address,uint256)(topic00xddf252ad…) on every non-zero value transfer to a different account: the top-level tx transfer, value-carryingCALL,SELFDESTRUCT, andCREATE/CREATE2. This reuses the existingeth_simulatenative-transfer machinery, re-gated to fire on the consensus path. Two emitter addresses are in play and 7708 does not replace one with the other: the consensus log fromSYSTEM_ADDRESSfires whenever 7708 is active, andeth_simulate's synthetic from the ERC-7528 native-token address0xeeee…is emitted alongside it whentraceTransfersis set — geth returns both. Note thatlogIndexcounts both, so a consumer dropping the0xeeee…entries sees only even indices.SYSTEM_ADDRESSwas already a function-local constant inprocess_requests.cpp, the sender the EIP-7002/7251 system calls execute as. 7708 needs the same address, so it is promoted to a shared constant rather than leaving two spellings of one consensus constant that can silently diverge.There is deliberately no Burn log
An earlier draft of EIP-7708 also specified
Burn(address,uint256)for ETH destroyed bySELFDESTRUCT, and this PR used to implement it. That draft then took EIP-8246 as a prerequisite and dropped the log, because after 8246 there is no burn left to report. EIP-8246 is #2512, also based onmain— it must merge first, so that by the time 7708 is live the burn never happens and the log would have no subject. geth and erigon are in the same configuration.Earlier revisions of this description called the Burn half a monad-specific extension. That was wrong twice over — it was standard 7708 at the time, and it is not in the EIP at all now.
Active, not inert
eip_7708_active()is>= MONAD_ETH_AMSTERDAMin both trait families, andMonadTraits<MONAD_NEXT>::evm_rev()is Amsterdam, so the rule is live on the configuration that ships. Earlier revisions of this description said it was held inert with the predicate returning false, and that Amsterdam sat aboveLATEST_SUPPORTED_EVM_FORKso nothing in the test matrix reached it. Neither is true any more: the tests run over an explicit two-element type list —MonadRevisionConstant<MONAD_NEXT>andEvmRevisionConstant<MONAD_ETH_AMSTERDAM>— naming both revisions where 7708 is active. They are named rather than derived fromLATEST_SUPPORTED_EVM_FORKso the suite does not depend on that constant having been advanced.Spec tests
The Amsterdam spec suite is switched off on this PR, not filtered:
MONAD_NEXT_amsterdam_excluded_testsis set toBlockchainTests.*, which tripsthe
WILL_FAILguard inCMakeLists.txt.The reason is a bundle-versus-code skew, not anything wrong here.
mainpinstests-monad_amsterdam@v0.2.0, which predates EIP-7708, so every fixture thatmoves ETH expects no
Transferlog — measured, 110 of them fail once the rule islive. The fixtures that do expect the logs are in v0.5.0 and later, and those
bundles' genesis headers carry
slotNumberandblockAccessListHash, so theyneed #2439's header RLP to load at all.
This is deliberately coarse and temporary: it also drops the ~550 fixtures
maincurrently passes. Restoring the real list — with a bundle generated with 7708
active, plus the entries #2407 and #2512 carry on their own branches — is the
follow-up PR's job. #2439 does the same thing for the same reason.
Unit tests are unaffected and are the real gate here:
test_state379 andtest_call_trace234 passing, both builds clean.Ordering
No longer a stack. This PR, #2407 (EIP-8024) and #2512 (EIP-8246) are all based
on
mainand independent of each other and of #2439 (SLOTNUM).One ordering constraint remains, and it is semantic rather than structural:
#2512 must merge before this PR. EIP-7708 lists
requires: 8246, and withoutit two
SELFDESTRUCTpaths still destroy ETH with no recipient — aTransferlog cannot express that, so 7708 would claim complete native-value logging while
those burns went unlogged.
🤖 Generated with Claude Code
https://claude.ai/code/session_01T2YcdwU1mj3Lqunp9Y7pqa